-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Collect lighthouse CI on each frontend change #4994
Conversation
Latest k6 run output1
Footnotes
|
Ah, what a shame, the free "temporary public storage" that is used by Lighthouse CI to support the GitHub app's functionality is (hopefully temporarily) broken: GoogleChrome/lighthouse-ci#1072 We can move forward with this PR as is, and spin up our own Lighthouse CI server on ECS in the meantime. In any case, collecting and running LHCI is the first step to either the publicly available free integration, or an Openverse-specific one where we run the server ourselves (if the free one doesn't come back). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to finally have this done, thank you!
I left a couple of questions, but they are non-blockers.
setup_python: false | ||
install_recipe: node-install | ||
|
||
- name: Run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reuse the built Nuxt app for this and for the k6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to push the .nuxt
directory as an artifact, I think, but it should be possible. It only takes 32-seconds though. Downloading and decompressing the artifact could take just as long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building makes more sense, considering the time it takes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nuxt 3 made it a lot better 🙂
run: | | ||
pnpm --package=@lhci/cli dlx lhci autorun --config .github/.lighthouserc.yml | ||
|
||
- name: Display Report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add it as a comment on the PR? Or do you think that would be too much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other actions out there that generate markdown tables, but they all looked pretty bad (too information dense) and the comments would be very long (similar to the old bundle size comments).
I think we should think of ways of making the results more noticeable, but those would probably come more from writing assertions so that the check fails when we get results we don't want. Lighthouse CI recommends you wait to add assertions until after you've had a chance to understand your results and make changes to improve them a bit first.
Ultimately, I think this action would be the best thing, once we are comfortable in our understanding of what we want from Lighthouse. That action requires setting up access to S3 for saving the files, which I'd prefer to do as a separate step to this one of just getting it running.
Fixes
Fixes #646 by @obulat
Description
This PR adds a new CI/CD job to collect Lighthouse CI results for each frontend change.
Testing Instructions
The new CI job must pass.... click in to see the nice table of results logged in the "Display Report" step
For now, that's as far as we'll go. It's already telling us some pretty useful (and worrisome!) things. In the future we can explore other methods of displaying results (e.g., https://www.foo.software/docs/lighthouse-check-github-action/intro, which requires a small amount of additional infrastructure work to get running, S3 and some IAM stuff). See this comment for more details about why other free and easy options are not currently available.
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin